Skip to content

End an external room child when its turn times out - #2635

Merged
Birfy merged 1 commit into
mainfrom
fix/2634-external-timeout-ends-child
Nov 7, 2026
Merged

End an external room child when its turn times out#2635
Birfy merged 1 commit into
mainfrom
fix/2634-external-timeout-ends-child

Conversation

@Birfy

@Birfy Birfy commented Nov 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Written-by: Dev

Fixes #2634

A timed-out room turn of an external participant now ends its child process instead of leaving it running, so the next turn starts a fresh process and the late answer to the timed-out turn cannot settle it.

Owner: Dev
Reviewer: Reviewer

Changes

  • core/src/host/room-participant-external.js — the timeout branch of runTurn() calls endChild(entry) before finish({ missed: true }), exactly as the abort branch already does. The timeout room report and the { missed: true } result are unchanged: a timeout is still silence, not a model failure.
  • core/test/room-external.test.js — a child that holds the first turn's answer until a marker appears and handles turns serially. The second turn must get its own answer or { missed: true }, never the first turn's text, and must reach a different process.
  • docs/subsystems/core-host.md — the host page says a timed-out participant is ended and the next turn starts a fresh process.
  • changelog.d/2634.fixed.md.

Ran at this tip:

$ cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/room-external.test.js
$ node scripts/ds-bot-patches.mjs check
$ node scripts/ds-bot-patches.mjs canonical
$ node core/scripts/build-client.mjs --check
$ npm --prefix core run lint

All green: 14/14 tests at this tip; with the timeout-path endChild(entry) reverted the new test fails with the second turn settled by the timed-out turn's answer to one. ds-bot-patches check and canonical are green, build-client --check is green, and lint reports 85 pre-existing warnings, 0 errors, with arch-docs: OK (6 changed file(s)).

Not run: the full npm run verify (another job was using this machine), the smoke test (no core/index.js or driver change) and the plugin suites (no plugin change).

Checkpoint

{
  "pr": 2635, "claim": "Dev", "issue": 2634,
  "summary": "A timed-out external room turn now ends its child process before the turn is finished, exactly as the abort path already did. Without that the next turn reused the still-running child and its late answer to the timed-out turn settled the new turn with the previous text. The timeout report and the { missed: true } result are unchanged, and the next turn starts a fresh process. A new test in core/test/room-external.test.js fails when the timeout-path endChild is reverted.",
  "files": ["core/src/host/room-participant-external.js", "core/test/room-external.test.js", "docs/subsystems/core-host.md", "changelog.d/2634.fixed.md", "patches/ds-bot/files/src/host/room-participant-external.js", "patches/ds-bot/files/test/room-external.test.js"],
  "checks": { "ran": "cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/room-external.test.js; node scripts/ds-bot-patches.mjs check; node scripts/ds-bot-patches.mjs canonical; node core/scripts/build-client.mjs --check; npm --prefix core run lint", "result": "green", "at": "09786e6b9e5b7426cc9d77b6f198861cb5dc415e" },
  "next": "none — ready for review"
}

@Birfy
End an external room child when its turn times out
09786e6 
A timed-out turn left the external participant's process running, so the
next turn reused it and the late line settled that new turn with the
previous turn's text. End the child on the timeout path as abort() already
does: the process is replaced on the next turn and its late line lands on
an entry with no turn.

Fixes #2634
@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 7, 2026
@Birfy Birfy self-assigned this Nov 7, 2026
@Birfy Birfy mentioned this pull request Nov 7, 2026
Closed
@Birfy

Birfy commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 09786e6
Written-by: Reviewer
Attestation: 09786e6 role=Reviewer

Ran at this tip:

$ cd /data/dsh/bot/workspace/r2635/tip/core && DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness TZ=UTC node --import ./test/harness-resolve.mjs --test test/room-external.test.js
# tests 14  # pass 14  # fail 0

$ cd /data/dsh/bot/workspace/r2635/red/core && DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness TZ=UTC node --import ./test/harness-resolve.mjs --test test/room-external.test.js
# tests 14  # pass 13  # fail 1
# not ok 9 - a timed-out turn never settles the next one with its late answer (#2634)
#   error: "the next turn was settled by the timed-out turn's late answer"
#   expected: 'answer to one'  actual: 'answer to one'  operator: 'notDeepStrictEqual'

$ cd /data/dsh/bot/workspace/r2635/tip && node scripts/ds-bot-patches.mjs check
# ok 10 patches apply cleanly in order
# ok core/ equals ds-bot/ + patches/ds-bot/

$ cmp core/src/host/room-participant-external.js patches/ds-bot/files/src/host/room-participant-external.js && cmp core/test/room-external.test.js patches/ds-bot/files/test/room-external.test.js
# both byte-identical (md5 ca61861… / 7aaca0e…)

Not run: full npm run verify, the smoke test, the plugin suites (out of scope for this line).

Red first: the tip's core/src/host/room-participant-external.js with only the timeout branch's endChild(entry) line deleted fails exactly as claimed (second turn settled by answer to one), and the restored tip is 14/14 green on the same machine.

Fix location and side effects: endChild(entry) sits before entry.state = 'timeout' / finish({ missed: true }), same as the abort branch. endChild sets entry.stopping = true and state = 'stopped', and reportExit returns early on either (core/src/host/room-participant-external.js:167), so no extra crashed/exited room line follows the single timeout report; { missed: true } is unchanged.

Fresh process is asserted, not just text: the child records start <pid> and turn <pid> <text>, and the test asserts starts()[1] !== starts()[0] plus answer !== 'answer to one'.

Scope: 6 files, exactly the change set (core/, its two patches/ds-bot/files/ mirrors, the owned docs/subsystems/core-host.md, and the changelog fragment); base 2e93b8c09174ddb6e843593c8e7b203348a7cb41. Docs: core/src/host/ is an owned path of core-host.md and the updated sentence matches the implementation (timed-out turn ends the child; next turn starts fresh). Checkpoint block: 1266 chars, files all in the diff, checks.at is this tip.

CI at the tip: tests-ok, merge-gate, Node 24, plugins, package green; enqueue-guard red only for "no Verifier report names the tip 09786e6" (this report is that one).

@github-actions github-actions Bot added the pass label Nov 7, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 7, 2026
Merged via the queue into main with commit 6a867a9 Nov 7, 2026
17 of 20 checks passed
@Birfy
Birfy deleted the fix/2634-external-timeout-ends-child branch November 7, 2026 19:42
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 7, 2026
@Birfy

Birfy commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator Author

Written-by: Enqueue

Merged through the merge queue: squash commit 6a867a9d02accde5811e0fcec02d0152f2485360 at 2026-11-07T19:42:37Z.

main head is that commit, and Test on the push to main is success (run 42407530148). Queue entry at 09786e6b9e5b7426cc9d77b6f198861cb5dc415e: merge-gate, enqueue-guard and Test on merge_group all success; every check at the tip terminal, no failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Birfy Birfy

Labels

pass

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug(host): a timed-out external room turn leaves its child running, so the next turn receives the stale answer

1 participant

@Birfy